-
Notifications
You must be signed in to change notification settings - Fork 411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add length validation to text input #3193
Conversation
2e4bca3
to
fcd1b69
Compare
</label> | ||
<input type="text" name="validpattern" | ||
ng-model="formData.validPattern" | ||
> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should go in the previous line (also two more times above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
<input type="text" name="validpattern" | ||
ng-model="formData.validPattern" | ||
> | ||
<i class="info-helper" title="{{ 'Validation pattern for input , see https://www.w3schools.com/jsref/jsref_obj_regexp.asp }}' | i18n }}"></i> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though w3schools isn't as awful as they used to be (but the still sell those phony "html certificates"), I don't want a link to them in Indico ;) (also, there shouldn't be a space before the ,
).
Maybe letting users specify regex patterns here is a bit overkill anyway...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed the link.
Using pattern validation can e.g. be used if you need special input from the registrants (e.g only uppercaser, no spaces, etc.)
<input type="number" name="maxlength" | ||
ng-model="formData.maxLength" | ||
> | ||
<i class="info-helper" title="{{ 'Maximimum allowed length' | i18n }}"></i> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to many spaces
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
fcd1b69
to
c85d19a
Compare
{{ 'Validation pattern' | i18n }} | ||
</label> | ||
<input type="text" name="validpattern" ng-model="formData.validPattern"> | ||
<i class="info-helper" title="{{ 'Validation pattern(a javascript regualr expression) }}' | i18n }}"></i> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing space before (
and a typo (regualr).
Anyway, I still don't like the idea of having the pattern option. Yes, it is powerful, but eventually we want server-side validation for this stuff and except when restricting yourself to a common subset of the regex features of HTML5/JS and Python, that won't be guaranteed to work on both sides...
Also, having user-provided regexps run on the server may be risky - usually there are some ways to write regular expressions that result in horrible performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I splitted this part out, as regexp validation is not too important for now.
ng-class="{ | ||
hasError: validationStarted && nestedForm.$invalid | ||
}"/> | ||
|
||
<span ng-show="validationStarted && nestedForm.$invalid && nestedForm.$error.minlength" class="error-message-tag"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double space before class
(same below)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
c85d19a
to
5ad5e30
Compare
6e5cc39
to
c213091
Compare
Ping... |
9ed5f8a
to
8e09652
Compare
This change allows to enforce minimum and maximum length limit for registration form text input fields.
8e09652
to
972375e
Compare
Don't worry, we didn't forget about it - just busy with other things ;) It's cleaned up (typos etc.) and merged now. Thanks for the PR! |
This change allows to enforce minimum and maximum length
limit and/or pattern restrictions for regform text input fields.